fix: tag rename capitalization#555
Conversation
openedx-webhooks
added
the
open-source-contribution
|
Thanks for the pull request, @jesperhodge! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
There was a problem hiding this comment.
Pull request overview
This PR fixes tag rename behavior so that updating a tag’s value with only capitalization changes (e.g., Science → science) is treated as a valid update rather than a duplicate-value error. This aligns the tagging API behavior with the Course Authoring taxonomy UI rename flow.
Changes:
- Add a regression test covering capitalization-only tag rename via the taxonomy tags update endpoint.
- Update tag value uniqueness validation to exclude the tag being updated from the “already exists” check.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tests/openedx_tagging/test_views.py |
Adds a test ensuring capitalization-only renames succeed (HTTP 200) and return the updated value. |
src/openedx_tagging/rest_api/v1/serializers.py |
Adjusts uniqueness validation to avoid flagging the tag itself as a duplicate during updates. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Just so I'm clear: Is the idea that we don't allow tags that differ just by case, i.e. I couldn't have both "Science" and "science" at the same time, but we do want to allow changing case, so "science" can be renamed "Science"? |
|
Hi @ormsbee ,
That is correct, at least according to Jenna's remarks in UAT. But we should not check that new value of tag A is different than old value of A. That leads to unexpected validation errors, such as the case here where someone wants to rename a tag and he gets an error. Regardless of whether he should get an error or not (according to UAT and common sense, no), throwing an error that says the new tag value is not unique - when in fact it is unique, just similar to the tag's old value - would be incorrect. |
Fixes openedx/modular-learning#274
Testing Instructions